[fix] 게시글 좋아요 500 error(deadlock) 해결 #334
Conversation
…like-pessimistic-lock
Walkthrough비관적 쓰기 잠금( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PostLikeService
participant PostHandler
participant Repo as Feed/Record/Vote Repo
participant DB as Database
participant NotificationService
Client->>PostLikeService: changeLikeStatusPost(command)
activate PostLikeService
PostLikeService->>PostHandler: findPostForUpdate(type, id)
activate PostHandler
PostHandler->>Repo: findByPostIdForUpdate(postId)
activate Repo
Repo->>DB: SELECT ... WHERE postId=:postId\nwith PESSIMISTIC_WRITE lock (timeout=3000ms)
activate DB
DB-->>Repo: row (locked)
deactivate DB
Repo-->>PostHandler: Optional<Entity>
deactivate Repo
PostHandler-->>PostLikeService: Entity
deactivate PostHandler
rect rgb(220,240,220)
Note over PostLikeService: Pre-update mutation (update like count & persist)
PostLikeService->>PostHandler: updatePost(post)
end
rect rgb(240,230,210)
Note over PostLikeService: Decision: like or unlike
alt Like
PostLikeService->>PostHandler: saveLike(...)
PostLikeService->>NotificationService: sendNotifications(...)
else Unlike
PostLikeService->>PostHandler: deleteLike(...)
end
end
PostLikeService-->>Client: success / error
deactivate PostLikeService
rect rgb(255,220,220)
Note over PostLikeService: On LockTimeoutException -> `@Recover` -> BusinessException(RESOURCE_LOCKED)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Test Results488 tests 488 ✅ 43s ⏱️ Results for commit e56da5c. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
src/main/java/konkuk/thip/roompost/adapter/out/persistence/repository/record/RecordJpaRepository.java (1)
21-23: 비관적 락 타임아웃 설정 추가 권장
PESSIMISTIC_WRITE락이 올바르게 적용되었습니다. 그러나 락 대기 시 무한 대기를 방지하기 위해@QueryHints로 타임아웃을 설정하는 것이 좋습니다.다음 diff를 적용하여 타임아웃을 추가하세요:
@Lock(LockModeType.PESSIMISTIC_WRITE) +@QueryHints(@QueryHint(name = "jakarta.persistence.lock.timeout", value = "3000")) @Query("SELECT r FROM RecordJpaEntity r WHERE r.postId = :postId") Optional<RecordJpaEntity> findByPostIdForUpdate(@Param("postId") Long postId);import 추가:
import jakarta.persistence.QueryHint; import org.springframework.data.jpa.repository.QueryHints;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
loadtest/feed/feed_like_concurrency_test1.js(1 hunks)loadtest/feed/feed_like_concurrency_test2.js(1 hunks)loadtest/feed/feed_like_concurrency_test3.js(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedJpaRepository.java(2 hunks)src/main/java/konkuk/thip/feed/application/port/out/FeedCommandPort.java(1 hunks)src/main/java/konkuk/thip/post/application/service/PostLikeService.java(1 hunks)src/main/java/konkuk/thip/post/application/service/handler/PostHandler.java(1 hunks)src/main/java/konkuk/thip/roompost/adapter/out/persistence/RecordCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/roompost/adapter/out/persistence/VoteCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/roompost/adapter/out/persistence/repository/record/RecordJpaRepository.java(2 hunks)src/main/java/konkuk/thip/roompost/adapter/out/persistence/repository/vote/VoteJpaRepository.java(2 hunks)src/main/java/konkuk/thip/roompost/application/port/out/RecordCommandPort.java(1 hunks)src/main/java/konkuk/thip/roompost/application/port/out/VoteCommandPort.java(1 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedChangeLikeStatusApiTest.java(1 hunks)src/test/java/konkuk/thip/feed/concurrency/FeedChangeLikeStatusConcurrencyTest.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomPostChangeLikeStatusApiTest.java(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-07-26T06:09:00.850Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서 Record와 Vote는 Room에 속하지만 Feed는 Room에 속하지 않는 구조이며, 댓글 작성 시 Record/Vote에 대해서만 사용자가 해당 Room의 참가자인지 검증이 필요하다.
Applied to files:
src/test/java/konkuk/thip/room/adapter/in/web/RoomPostChangeLikeStatusApiTest.java
📚 Learning: 2025-07-03T03:05:05.031Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
Applied to files:
src/main/java/konkuk/thip/roompost/application/port/out/RecordCommandPort.javasrc/main/java/konkuk/thip/feed/application/port/out/FeedCommandPort.javasrc/main/java/konkuk/thip/roompost/application/port/out/VoteCommandPort.java
📚 Learning: 2025-09-01T13:18:13.652Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 287
File: src/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.java:8-14
Timestamp: 2025-09-01T13:18:13.652Z
Learning: seongjunnoh는 JpaRepository의 findById 메서드 재정의보다는 도메인별 명시적 메서드(findByUserId, findByRoomId 등)를 정의하여 Hibernate Filter 적용을 보장하는 방식을 선호하며, 이를 통해 더 안전하고 의도가 명확한 코드 구조를 구축한다.
Applied to files:
src/main/java/konkuk/thip/roompost/adapter/out/persistence/RecordCommandPersistenceAdapter.java
📚 Learning: 2025-07-14T14:19:38.796Z
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 75
File: src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteQueryRepositoryImpl.java:50-83
Timestamp: 2025-07-14T14:19:38.796Z
Learning: Vote와 VoteItem 엔티티는 자주 함께 사용되므로, N+1 문제를 방지하기 위해 양방향 매핑과 fetch join을 고려하는 것이 좋습니다. 특히 기록장 조회 API 등에서도 함께 사용될 가능성이 높습니다.
Applied to files:
src/main/java/konkuk/thip/roompost/adapter/out/persistence/VoteCommandPersistenceAdapter.javasrc/main/java/konkuk/thip/roompost/application/port/out/VoteCommandPort.java
🧬 Code graph analysis (4)
loadtest/feed/feed_like_concurrency_test2.js (2)
loadtest/feed/feed_like_concurrency_test1.js (8)
BASE_URL(5-5)setup(21-34)tokens(22-22)likeStatus(23-23)userId(26-26)res(27-27)res(56-56)token(38-38)loadtest/feed/feed_like_concurrency_test3.js (10)
BASE_URL(5-5)setup(30-43)maxVUs(32-32)tokens(33-33)userId(36-36)res(37-37)res(56-56)res(95-95)token(48-48)token(73-73)
loadtest/feed/feed_like_concurrency_test1.js (2)
loadtest/feed/feed_like_concurrency_test2.js (10)
BASE_URL(5-5)options(8-20)options(8-20)setup(23-38)tokens(26-26)likeStatus(27-27)userId(30-30)res(31-31)res(60-60)token(42-42)loadtest/feed/feed_like_concurrency_test3.js (11)
BASE_URL(5-5)options(8-27)options(8-27)setup(30-43)tokens(33-33)userId(36-36)res(37-37)res(56-56)res(95-95)token(48-48)token(73-73)
loadtest/feed/feed_like_concurrency_test3.js (2)
loadtest/feed/feed_like_concurrency_test1.js (10)
BASE_URL(5-5)options(9-18)options(9-18)setup(21-34)tokens(22-22)userId(26-26)res(27-27)res(56-56)token(38-38)handleSummary(75-79)loadtest/feed/feed_like_concurrency_test2.js (11)
BASE_URL(5-5)options(8-20)options(8-20)setup(23-38)maxVUs(25-25)tokens(26-26)userId(30-30)res(31-31)res(60-60)token(42-42)handleSummary(79-83)
src/test/java/konkuk/thip/feed/concurrency/FeedChangeLikeStatusConcurrencyTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(35-417)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (10)
src/main/java/konkuk/thip/post/application/service/handler/PostHandler.java (1)
34-40: LGTM! 일관된 패턴으로 update용 조회 메서드 추가기존
findPost메서드와 동일한 구조로 update-lock 경로를 제공하여 코드 일관성이 유지됩니다.src/main/java/konkuk/thip/roompost/adapter/out/persistence/RecordCommandPersistenceAdapter.java (1)
59-63: LGTM! Adapter 레이어의 update-lock 경로 구현Repository의
findByPostIdForUpdate를 호출하고 도메인 엔티티로 매핑하는 일관된 패턴입니다.src/main/java/konkuk/thip/roompost/application/port/out/VoteCommandPort.java (1)
23-31: LGTM! Port 인터페이스의 update-lock 경로 추가기존
findById/getByIdOrThrow패턴과 일관되게 update용 메서드를 제공합니다. default 메서드를 통해 예외 처리를 캡슐화한 것도 좋습니다.src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java (1)
47-51: LGTM! Feed Adapter의 update-lock 경로 구현다른 도메인(Record, Vote)과 일관된 패턴으로 구현되어 있습니다.
src/main/java/konkuk/thip/feed/application/port/out/FeedCommandPort.java (1)
15-23: LGTM! Feed Port의 update-lock 경로 추가다른 CommandPort들(VoteCommandPort, RecordCommandPort)과 동일한 패턴으로 일관성이 유지됩니다.
src/main/java/konkuk/thip/roompost/adapter/out/persistence/VoteCommandPersistenceAdapter.java (1)
84-88: LGTM! Vote Adapter의 update-lock 경로 구현다른 Adapter들과 일관된 패턴으로 구현되어 있습니다.
src/main/java/konkuk/thip/roompost/application/port/out/RecordCommandPort.java (1)
18-26: LGTM! Record Port의 update-lock 경로 추가다른 CommandPort들과 동일한 패턴으로 일관성이 잘 유지됩니다.
src/test/java/konkuk/thip/feed/concurrency/FeedChangeLikeStatusConcurrencyTest.java (1)
32-58: 테스트 설정이 적절합니다.동시성 테스트를 위한 기본 설정이 올바르게 구성되어 있습니다.
src/main/java/konkuk/thip/roompost/adapter/out/persistence/repository/vote/VoteJpaRepository.java (1)
21-23: 비관적 락 구현이 올바릅니다.
PESSIMISTIC_WRITE락을 사용한findByPostIdForUpdate메서드가 올바르게 구현되었습니다. 데드락 해결을 위한 적절한 접근 방식입니다.src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedJpaRepository.java (1)
21-23: 비관적 락 구현이 올바릅니다.
PESSIMISTIC_WRITE락을 사용한findByPostIdForUpdate메서드가 올바르게 구현되었습니다. Feed 엔티티에 대한 동시성 제어가 적절하게 적용되었습니다.
src/test/java/konkuk/thip/feed/adapter/in/web/FeedChangeLikeStatusApiTest.java
Outdated
Show resolved
Hide resolved
| @Test | ||
| public void concurrentLikeToggleTest() throws InterruptedException { | ||
|
|
||
| int threadCount = 2; | ||
| int repeat = 10; // 스레드별 몇 번 반복할지 | ||
| ExecutorService executor = Executors.newFixedThreadPool(threadCount); | ||
| CountDownLatch latch = new CountDownLatch(threadCount * repeat); | ||
|
|
||
| AtomicInteger successCount = new AtomicInteger(); | ||
| AtomicInteger failCount = new AtomicInteger(); | ||
|
|
||
| // 각 스레드별로 현재 상태(true/false)를 관리하기 위한 배열 | ||
| boolean[] likeStatus = new boolean[threadCount]; | ||
|
|
||
| for (int i = 0; i < threadCount; i++) { | ||
| final int userIndex = i; | ||
| executor.submit(() -> { | ||
| likeStatus[userIndex] = true; | ||
| for (int r = 0; r < repeat; r++) { | ||
| boolean isLike = likeStatus[userIndex]; | ||
| try { | ||
| // 각 스레드별로 서로 다른 user를 사용하도록 user1, user2 분기 처리 | ||
| Long userId = (userIndex == 0) ? user1.getUserId() : user2.getUserId(); | ||
|
|
||
| postLikeService.changeLikeStatusPost( | ||
| new PostIsLikeCommand(userId, feed.getPostId(), PostType.FEED, isLike) | ||
| ); | ||
| successCount.getAndIncrement(); | ||
| // 성공했을 때만 현재 상태를 반전 | ||
| likeStatus[userIndex] = !likeStatus[userIndex]; | ||
| } catch (Exception e) { | ||
| log.error(e.getMessage(), e); | ||
| failCount.getAndIncrement(); | ||
| } finally { | ||
| latch.countDown(); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| latch.await(); | ||
| executor.shutdown(); | ||
|
|
||
| // then | ||
| assertAll( | ||
| () -> assertThat(successCount.get()).isEqualTo(threadCount * repeat), | ||
| () -> assertThat(failCount.get()).isEqualTo(0) | ||
| ); | ||
| } |
There was a problem hiding this comment.
최종 데이터베이스 상태 검증이 누락되었습니다.
현재 테스트는 작업 성공 횟수만 확인하지만, 실제 데이터베이스의 최종 상태(좋아요 개수, PostLike 엔티티 존재 여부)를 검증하지 않습니다. 각 사용자가 10번씩 토글하므로 최종적으로 두 사용자 모두 좋아요 상태여야 하며, feed의 likeCount는 2여야 합니다.
테스트 마지막에 다음 검증을 추가하는 것을 권장합니다:
latch.await();
executor.shutdown();
+
+// 최종 데이터베이스 상태 검증
+FeedJpaEntity finalFeed = feedJpaRepository.findById(feed.getPostId()).orElseThrow();
+long finalLikeCount = postLikeQueryPort.countByPostId(feed.getPostId());
// then
assertAll(
() -> assertThat(successCount.get()).isEqualTo(threadCount * repeat),
- () -> assertThat(failCount.get()).isEqualTo(0)
+ () -> assertThat(failCount.get()).isEqualTo(0),
+ () -> assertThat(finalFeed.getLikeCount()).isEqualTo(2),
+ () -> assertThat(finalLikeCount).isEqualTo(2)
);🤖 Prompt for AI Agents
In
src/test/java/konkuk/thip/feed/concurrency/FeedChangeLikeStatusConcurrencyTest.java
around lines 61 to 109, the test only asserts success/fail counters but misses
verifying the final DB state; update the end of the test to reload the feed and
post-like data from the repository (or service) after
latch.await()/executor.shutdown(), then assert that each user has a PostLike
record with isLike==true and that feed.getLikeCount() (reloaded) equals 2 (or
the expected value), ensuring you use repository/service methods to fetch fresh
entities (and wrap reload/asserts in the same transactional/test context if
needed).
There was a problem hiding this comment.
코리가 말하는 것처럼 최종적으로 마지막에 DB 정합성이 깨지는지 확인하는 것도 좋을 것 같네욥
There was a problem hiding this comment.
@buzz0331, 좋은 지적입니다! 동시성 테스트에서 최종 DB 정합성 검증을 추가하는 것이 중요하네요.
다만 테스트 로직을 분석해보니, 각 사용자가 true로 시작해서 10번 토글하므로:
- 1번째: 좋아요 추가
- 2번째: 좋아요 취소
- 3번째: 좋아요 추가
- ...
- 10번째: 좋아요 취소
최종적으로 두 사용자 모두 좋아요 취소 상태가 되어, likeCount는 0이 되어야 합니다.
다음과 같이 최종 DB 상태 검증을 추가하는 것을 권장합니다:
latch.await();
executor.shutdown();
// 최종 데이터베이스 상태 검증
FeedJpaEntity finalFeed = feedJpaRepository.findById(feed.getPostId()).orElseThrow();
// then
assertAll(
() -> assertThat(successCount.get()).isEqualTo(threadCount * repeat),
() -> assertThat(failCount.get()).isEqualTo(0),
() -> assertThat(finalFeed.getLikeCount()).isEqualTo(0),
() -> assertThat(finalFeed.getLikeCount()).as("DB의 likeCount와 실제 상태가 일치해야 함").isEqualTo(0)
);더 완벽한 검증을 위해서는 PostLike 엔티티의 실제 개수도 확인하면 좋습니다. 만약 PostLikeCrudRepository 같은 레포지토리가 있다면:
@Autowired private PostLikeCrudRepository postLikeCrudRepository;
// 테스트 마지막에
long actualPostLikeCount = postLikeCrudRepository.countByPostIdAndIsLike(feed.getPostId(), true);
assertThat(actualPostLikeCount).isEqualTo(0);이렇게 하면 동시성 제어가 제대로 작동해서 데이터 정합성이 유지되는지 확실하게 검증할 수 있습니다! 👍
🧠 Learnings used
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 309
File: src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java:36-44
Timestamp: 2025-09-23T08:31:05.161Z
Learning: buzz0331은 기술적 이슈에 대해 실용적인 해결책을 제시하면서도 과도한 엔지니어링을 피하는 균형감을 선호한다. 복잡도 대비 실제 발생 가능성을 고려하여 "굳이" 불필요한 솔루션보다는 심플함을 유지하는 것을 중요하게 생각한다.
src/test/java/konkuk/thip/room/adapter/in/web/RoomPostChangeLikeStatusApiTest.java
Outdated
Show resolved
Hide resolved
src/test/java/konkuk/thip/room/adapter/in/web/RoomPostChangeLikeStatusApiTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
수고하셨습니다~ 문서가 아주 깔끔하네여 👍🏻 👍🏻
희진님도 찾아보셨겠지만 업데이트 충돌이 잦은 환경에서는 낙관락보다는 비즈니스 로직 성공을 반드시 보장해주는 비관락을 권장하는 것으로 알고있어욥
다만, 현재 희진님이 작성해두신 노션을 확인해보니 비관락을 걸었을때 대략 300 RPS까지 오르고 요청 실패가 한번도 없었던 것으로 확인되는데 비관락이 기본적으로 명시적으로 락을 걸면서 리소스 접근 자체를 막는거다보니 더 높은 RPS에서 과연 request time out이 나지 않고 버틸 수 있을지 궁금해지네요.
RPS를 조금 비현실적이더라도 비약적으로 높여보면서 request time out이 나는 지점을 확인하고 그 다음 단계인 Redis 도입, 이벤트 처리를 적용시켜보는 것도 좋을 것 같다는 생각입니다!
| @Test | ||
| public void concurrentLikeToggleTest() throws InterruptedException { | ||
|
|
||
| int threadCount = 2; | ||
| int repeat = 10; // 스레드별 몇 번 반복할지 | ||
| ExecutorService executor = Executors.newFixedThreadPool(threadCount); | ||
| CountDownLatch latch = new CountDownLatch(threadCount * repeat); | ||
|
|
||
| AtomicInteger successCount = new AtomicInteger(); | ||
| AtomicInteger failCount = new AtomicInteger(); | ||
|
|
||
| // 각 스레드별로 현재 상태(true/false)를 관리하기 위한 배열 | ||
| boolean[] likeStatus = new boolean[threadCount]; | ||
|
|
||
| for (int i = 0; i < threadCount; i++) { | ||
| final int userIndex = i; | ||
| executor.submit(() -> { | ||
| likeStatus[userIndex] = true; | ||
| for (int r = 0; r < repeat; r++) { | ||
| boolean isLike = likeStatus[userIndex]; | ||
| try { | ||
| // 각 스레드별로 서로 다른 user를 사용하도록 user1, user2 분기 처리 | ||
| Long userId = (userIndex == 0) ? user1.getUserId() : user2.getUserId(); | ||
|
|
||
| postLikeService.changeLikeStatusPost( | ||
| new PostIsLikeCommand(userId, feed.getPostId(), PostType.FEED, isLike) | ||
| ); | ||
| successCount.getAndIncrement(); | ||
| // 성공했을 때만 현재 상태를 반전 | ||
| likeStatus[userIndex] = !likeStatus[userIndex]; | ||
| } catch (Exception e) { | ||
| log.error(e.getMessage(), e); | ||
| failCount.getAndIncrement(); | ||
| } finally { | ||
| latch.countDown(); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| latch.await(); | ||
| executor.shutdown(); | ||
|
|
||
| // then | ||
| assertAll( | ||
| () -> assertThat(successCount.get()).isEqualTo(threadCount * repeat), | ||
| () -> assertThat(failCount.get()).isEqualTo(0) | ||
| ); | ||
| } |
There was a problem hiding this comment.
코리가 말하는 것처럼 최종적으로 마지막에 DB 정합성이 깨지는지 확인하는 것도 좋을 것 같네욥
seongjunnoh
left a comment
seongjunnoh
left a comment
There was a problem hiding this comment.
확인했습니다! 그런데 비관적 락에 타임아웃 관련해서 명시적으로 설정 하지 않으셨던데, 비즈니스 로직 상 게시글 좋아요에 그렇게 많은 부하가 발생할라나? 라는 의도로 락 타임아웃 설정은 제외하신 걸까요??
mysql은 타임아웃의 디폴트 값이 50초라고 알고 있는데, jpa 환경에서도 이게 유지되는지는 확인해봐야 할 것 같습니다!
개인적으로 락을 걸 때 혹시 모를 장애상황을 대비해서 타임아웃 + 이후 처리(디스코드 알림 or 보상 재시도 등등) 를 고려하는게 좋지 않나 생각합니다!
제가 진행해본 부하테스트(최대 vu200명이 동시에 좋아요 요청/취소를 반복하는경우)로는 락 타임아웃의 요청이 1건도 발생하지않아서 고려하지않았습니다!! 지금은 성준님이 작성한 부하 테스트 스크립트처럼 어떤 특정상황에 좋아요요청이 동시에 몰릴경우 부하 테스트를 진행하려고하는데 이테스트로 리퀘스트 타임아웃이 나는 지점을 확인해보고 그 후 처리도 고려해보겠습니닷!! |
… 꺠지는 테스트 코드 수정 - 테스트 클래스에 @ Transactional 사용 대신 명시적으로 @ AfterEach 사용해 사용한 엔티티 삭제 (#322)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/konkuk/thip/post/application/service/PostLikeService.java (1)
60-74: 좋아요 수 업데이트 순서가 검증 이전에 수행됩니다.이전 리뷰에서 지적된 것처럼, 좋아요 수 업데이트(60-62줄)가 검증(66, 72줄) 이전에 수행됩니다. 트랜잭션 롤백으로 데이터 일관성은 유지되지만, 로직 흐름상 검증 후 상태 변경이 더 자연스럽습니다.
비관적 락 환경에서 트랜잭션이 롤백되면 복구되므로 기능적으로는 문제없지만, 가독성과 유지보수성을 위해 순서 변경을 권장합니다:
// 2. 유저가 해당 게시물에 대해 좋아요 했는지 조회 boolean alreadyLiked = postLikeQueryPort.isLikedPostByUser(command.userId(), command.postId()); -// 3. 게시물 좋아요 수 업데이트 -post.updateLikeCount(postCountService,command.isLike()); -postHandler.updatePost(command.postType(), post); - -// 4. 좋아요 상태변경 +// 3. 좋아요 상태변경 if (command.isLike()) { postLikeAuthorizationValidator.validateUserCanLike(alreadyLiked); postLikeCommandPort.save(command.userId(), command.postId(),command.postType()); } else { postLikeAuthorizationValidator.validateUserCanUnLike(alreadyLiked); postLikeCommandPort.delete(command.userId(), command.postId()); } + +// 4. 게시물 좋아요 수 업데이트 +post.updateLikeCount(postCountService, command.isLike()); +postHandler.updatePost(command.postType(), post);
🧹 Nitpick comments (5)
src/test/java/konkuk/thip/room/adapter/in/web/RoomPostChangeLikeStatusApiTest.java (2)
168-170: 테스트 안정성을 위해saveAndFlush()사용을 권장합니다.
save()만으로는 MockMvc 요청 전에 DB에 즉시 반영되지 않을 수 있습니다. 테스트 전제 조건("다른 유저가 이미 좋아요한 상태")을 확실히 보장하려면saveAndFlush()를 사용하세요.다음 diff를 적용하세요:
- recordJpaRepository.save(record); + recordJpaRepository.saveAndFlush(record);
252-254: 테스트 안정성을 위해saveAndFlush()사용을 권장합니다.Record 테스트와 동일하게,
save()만으로는 MockMvc 요청 시점에 변경사항이 DB에 반영되지 않을 수 있습니다. 테스트 전제 조건의 확실한 반영을 위해saveAndFlush()를 사용하세요.다음 diff를 적용하세요:
- voteJpaRepository.save(vote); + voteJpaRepository.saveAndFlush(vote);src/main/java/konkuk/thip/ThipServerApplication.java (1)
7-7: 사용되지 않는 import입니다.
EnableRetryimport가 추가되었지만, 실제로 이 클래스에서는 사용되지 않습니다.RetryConfig.java에서 이미@EnableRetry를 활성화하고 있으므로 이 import는 제거해도 됩니다.-import org.springframework.retry.annotation.EnableRetry;src/main/java/konkuk/thip/post/application/service/PostLikeService.java (1)
79-82: @recover 메서드에서 원본 예외 정보가 손실됩니다.재시도 실패 시 원본 예외를 로깅하지 않고 새로운
BusinessException만 던지면, 실제 실패 원인 추적이 어렵습니다. 디버깅을 위해 원본 예외를 로깅하거나 cause로 전달하는 것을 권장합니다.+import lombok.extern.slf4j.Slf4j; + +@Slf4j @Service @RequiredArgsConstructor public class PostLikeService implements PostLikeUseCase { // ... @Recover public PostIsLikeResult recover(Exception e, PostIsLikeCommand command) { + log.warn("좋아요 처리 재시도 실패 - postId: {}, userId: {}, isLike: {}", + command.postId(), command.userId(), command.isLike(), e); throw new BusinessException(ErrorCode.RESOURCE_LOCKED); }loadtest/feed/feed-like-load-test.js (1)
161-166: import 문이 파일 하단에 위치해 있습니다.k6에서는 동작하지만, 가독성을 위해 다른 import 문들(lines 2-4)과 함께 파일 상단으로 이동하는 것이 좋습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
build.gradle(1 hunks)loadtest/feed/feed-like-load-test.js(1 hunks)src/main/java/konkuk/thip/ThipServerApplication.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/config/RetryConfig.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedJpaRepository.java(2 hunks)src/main/java/konkuk/thip/post/application/service/PostLikeService.java(3 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedChangeLikeStatusApiTest.java(3 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomPostChangeLikeStatusApiTest.java(4 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomPostChangeLikeStatusControllerTest.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedJpaRepository.java
- src/test/java/konkuk/thip/feed/adapter/in/web/FeedChangeLikeStatusApiTest.java
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서 Record와 Vote는 Room에 속하지만 Feed는 Room에 속하지 않는 구조이며, 댓글 작성 시 Record/Vote에 대해서만 사용자가 해당 Room의 참가자인지 검증이 필요하다.
📚 Learning: 2025-08-16T10:52:23.225Z
Learnt from: hd0rable
Repo: THIP-TextHip/THIP-Server PR: 238
File: src/main/java/konkuk/thip/comment/adapter/out/persistence/repository/CommentQueryRepositoryImpl.java:190-216
Timestamp: 2025-08-16T10:52:23.225Z
Learning: THIP 프로젝트에서 User와 Alias는 1:1 매핑 관계이며, 데이터베이스 제약조건도 nullable = false로 설정되어 있어 모든 사용자는 반드시 alias를 가진다.
Applied to files:
src/test/java/konkuk/thip/room/adapter/in/web/RoomPostChangeLikeStatusControllerTest.java
📚 Learning: 2025-10-13T08:39:43.833Z
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 323
File: build.gradle:102-104
Timestamp: 2025-10-13T08:39:43.833Z
Learning: Spring AI 1.0.0-M6에서 Google AI Gemini 전용 스타터가 빈 등록에 실패하는 경우, spring-ai-openai-spring-boot-starter를 사용하고 외부 설정(환경 변수 등)으로 spring.ai.openai.base-url을 Google의 OpenAI 호환 엔드포인트로, spring.ai.openai.api-key를 Google Cloud 액세스 토큰으로, spring.ai.openai.chat.options.model을 Gemini 모델명으로 지정하여 우회할 수 있습니다.
Applied to files:
build.gradle
📚 Learning: 2025-07-26T06:09:00.850Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서 Record와 Vote는 Room에 속하지만 Feed는 Room에 속하지 않는 구조이며, 댓글 작성 시 Record/Vote에 대해서만 사용자가 해당 Room의 참가자인지 검증이 필요하다.
Applied to files:
src/test/java/konkuk/thip/room/adapter/in/web/RoomPostChangeLikeStatusApiTest.java
📚 Learning: 2025-06-29T09:17:39.787Z
Learnt from: hd0rable
Repo: THIP-TextHip/THIP-Server PR: 36
File: build.gradle:30-30
Timestamp: 2025-06-29T09:17:39.787Z
Learning: THIP-Server 프로젝트에서 Redis 의존성(spring-boot-starter-data-redis)은 향후 사용 예정으로 의도적으로 추가되었음. 현재는 사용하지 않지만 추후 개발에서 활용할 계획임.
Applied to files:
src/main/java/konkuk/thip/ThipServerApplication.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (8)
src/test/java/konkuk/thip/room/adapter/in/web/RoomPostChangeLikeStatusControllerTest.java (1)
75-82: LGTM!테스트 격리를 위한 명시적 정리 로직이 올바르게 구현되었습니다.
deleteAllInBatch()사용으로 효율적이며, 삭제 순서도 외래 키 제약조건(roomParticipant/record → room → book, user)을 준수합니다.src/test/java/konkuk/thip/room/adapter/in/web/RoomPostChangeLikeStatusApiTest.java (1)
84-94: LGTM!통합 테스트를 위한 정리 로직이 올바르게 구현되었습니다. 삭제 순서가 복잡한 외래 키 관계(postLike → feed/record/vote → room → book/user)를 정확히 반영하고 있습니다.
build.gradle (1)
104-106: LGTM!
spring-retry의존성 추가가 적절합니다.spring-boot-starter-aop가 이미 포함되어 있어@Retryable기능이 정상 동작할 것입니다.src/main/java/konkuk/thip/config/RetryConfig.java (1)
1-9: LGTM!Spring Retry 설정이 적절합니다.
proxyTargetClass = true설정으로 인터페이스 기반이 아닌 클래스에서도@Retryable이 정상 동작합니다.src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
34-34: LGTM!
RESOURCE_LOCKED에러 코드 추가가 적절합니다. HTTP 423 상태 코드와 50200 코드 번호가 기존 규칙을 따르고 있습니다.src/main/java/konkuk/thip/post/application/service/PostLikeService.java (2)
44-49: Retry 설정이 적절합니다.비관적 락 경합 시 재시도 로직이 잘 구성되어 있습니다.
REQUIRES_NEWpropagation과 함께 사용하여 각 재시도가 새로운 트랜잭션에서 실행됩니다. exponential backoff와 random jitter 설정도 적절합니다.
70-70: 주석 처리된 알림 전송 코드 확인이 필요합니다.
sendNotifications(command)호출이 주석 처리되어 있습니다. 의도적인 비활성화인지, 아니면 추후 활성화할 TODO인지 명확히 해주세요. 의도적이라면 주석으로 이유를 남기거나, 불필요하다면 코드를 제거하는 것이 좋습니다.loadtest/feed/feed-like-load-test.js (1)
122-122: 좋아요 요청 body의type값이 문자열입니다.
{ type: 'true' }형태로 문자열'true'를 전송하고 있습니다. API가 boolean이 아닌 문자열을 기대하는지 확인해주세요. 일반적으로 boolean 값이 예상됩니다.-const body = JSON.stringify({ type: 'true' }); +const body = JSON.stringify({ type: true });
loadtest/feed/feed-like-load-test.js
Outdated
| case ERR.RESOURCE_LOCKED: | ||
| http423.add(1); | ||
| fail_RESOURCE_LOCKED.add(1); | ||
| break; |
There was a problem hiding this comment.
정의되지 않은 메트릭 http423 사용으로 런타임 에러가 발생합니다.
Line 144에서 http423.add(1)을 호출하지만, 이 Counter는 정의되지 않았습니다. k6 실행 시 ReferenceError가 발생합니다.
const fail_RESOURCE_LOCKED = new Counter('fail_RESOURCE_LOCKED');
const fail_OTHER_4XX = new Counter('fail_OTHER_4XX');
+const http423 = new Counter('feed_like_423');또는 http423.add(1) 호출을 제거하고 fail_RESOURCE_LOCKED.add(1)만 유지:
case ERR.RESOURCE_LOCKED:
- http423.add(1);
fail_RESOURCE_LOCKED.add(1);
break;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case ERR.RESOURCE_LOCKED: | |
| http423.add(1); | |
| fail_RESOURCE_LOCKED.add(1); | |
| break; | |
| case ERR.RESOURCE_LOCKED: | |
| fail_RESOURCE_LOCKED.add(1); | |
| break; |
🤖 Prompt for AI Agents
In loadtest/feed/feed-like-load-test.js around lines 143 to 146, the code calls
an undefined metric http423 (http423.add(1)) causing a ReferenceError; fix by
either (A) defining http423 as a Counter near the other HTTP-status counters
(same naming/initialization pattern used in the file) so the add call is valid,
or (B) remove the http423.add(1) call and keep only fail_RESOURCE_LOCKED.add(1)
if you don't intend to track a separate 423 counter; ensure whichever option you
choose follows the file's existing metric declaration style and is consistently
used elsewhere.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/java/konkuk/thip/post/application/service/PostLikeService.java (1)
60-74: 검증 전 상태 변경으로 트랜잭션 무결성 위험이 있습니다.이 문제는 이전 리뷰에서도 지적되었으나 여전히 수정되지 않았습니다. 현재 코드는 다음 순서로 동작합니다:
- 60-62줄: 좋아요 수를 먼저 업데이트
- 66, 72줄: 그 후에 검증 수행
이로 인해 발생하는 문제:
- 이미 좋아요한 상태에서 다시 좋아요를 시도하면 66줄에서 예외가 발생하지만, 그 전에 이미 카운트가 증가되어 있습니다(트랜잭션 롤백으로 복구되지만 로직상 부자연스럽습니다)
- 검증 실패 시점과 상태 변경 시점이 뒤섞여 코드 이해가 어렵습니다
- 향후 추가 로직이 실패할 경우 일관성 문제가 발생할 수 있습니다
다음과 같이 순서를 변경하세요:
// 2. 유저가 해당 게시물에 대해 좋아요 했는지 조회 boolean alreadyLiked = postLikeQueryPort.isLikedPostByUser(command.userId(), command.postId()); -// 3. 게시물 좋아요 수 업데이트 -post.updateLikeCount(postCountService,command.isLike()); -postHandler.updatePost(command.postType(), post); - -// 4. 좋아요 상태변경 +// 3. 좋아요 상태변경 if (command.isLike()) { postLikeAuthorizationValidator.validateUserCanLike(alreadyLiked); postLikeCommandPort.save(command.userId(), command.postId(),command.postType()); // 좋아요 푸쉬알림 전송 sendNotifications(command); } else { postLikeAuthorizationValidator.validateUserCanUnLike(alreadyLiked); postLikeCommandPort.delete(command.userId(), command.postId()); } + +// 4. 게시물 좋아요 수 업데이트 +post.updateLikeCount(postCountService, command.isLike()); +postHandler.updatePost(command.postType(), post); return PostIsLikeResult.of(post.getId(), command.isLike());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/konkuk/thip/post/application/service/PostLikeService.java(4 hunks)src/test/java/konkuk/thip/feed/concurrency/FeedChangeLikeStatusConcurrencyTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/konkuk/thip/feed/concurrency/FeedChangeLikeStatusConcurrencyTest.java
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서 Record와 Vote는 Room에 속하지만 Feed는 Room에 속하지 않는 구조이며, 댓글 작성 시 Record/Vote에 대해서만 사용자가 해당 Room의 참가자인지 검증이 필요하다.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
src/main/java/konkuk/thip/post/application/service/PostLikeService.java (1)
53-53: 비관적 락 적용이 적절합니다.
findPostForUpdate를 사용하여 비관적 락을 적용한 것은 데드락 문제 해결을 위한 핵심 변경사항입니다.
| @Retryable( | ||
| notRecoverable = { BusinessException.class, InvalidStateException.class}, | ||
| maxAttempts = 3, | ||
| backoff = @Backoff(delay = 100, multiplier = 2, maxDelay = 500, random = true) | ||
| ) | ||
| @Transactional(propagation = Propagation.REQUIRES_NEW) |
There was a problem hiding this comment.
재시도 및 트랜잭션 설정을 재검토하세요.
현재 구성에서 여러 문제가 있습니다:
-
notRecoverable 설정 불일치:
PessimisticLockException,LockTimeoutException등 DB 락 관련 예외는 notRecoverable 목록에 없어 재시도되지만, 최종적으로 recover 메서드에서RESOURCE_LOCKED로 변환됩니다. 반면 비즈니스 로직 예외(BusinessException)는 재시도되지 않습니다. 이는 일관성 없는 동작을 야기합니다. -
Backoff 시간 부족 가능성: 총 재시도 시간이 ~800ms(100ms + 200ms + 500ms)로 짧습니다. 실제 락 경합 상황에서 다른 트랜잭션이 락을 더 오래 보유할 수 있어 재시도 창이 부족할 수 있습니다. PR 댓글에서 언급한 MySQL의 기본 락 타임아웃(~50초)과 비교하면 매우 짧습니다.
-
REQUIRES_NEW의 영향: 각 재시도마다 새 트랜잭션이 생성됩니다. 첫 시도에서 락 획득 → 카운트 업데이트 → 검증 실패 → 롤백 후, 다음 재시도는 완전히 새로운 트랜잭션으로 시작하여 DB 부하가 증가합니다.
-
명시적 락 타임아웃 미설정: PR 댓글에서 언급된 대로, JPA 수준에서 명시적 락 타임아웃을 설정하지 않았습니다. 이로 인해 데이터베이스 기본값에 의존하게 되며, 예상치 못한 긴 대기 시간이 발생할 수 있습니다.
다음 개선사항을 고려하세요:
@Retryable(
- notRecoverable = { BusinessException.class, InvalidStateException.class},
+ retryFor = {PessimisticLockException.class, CannotAcquireLockException.class},
+ noRetryFor = {BusinessException.class, InvalidStateException.class},
maxAttempts = 3,
- backoff = @Backoff(delay = 100, multiplier = 2, maxDelay = 500, random = true)
+ backoff = @Backoff(delay = 200, multiplier = 2, maxDelay = 2000, random = true)
)추가로, @Lock(LockModeType.PESSIMISTIC_WRITE) 사용 시 명시적 타임아웃을 설정하는 것을 권장합니다:
@QueryHints({@QueryHint(name = "javax.persistence.lock.timeout", value = "3000")})🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/post/application/service/PostLikeService.java
around lines 44-49, adjust the retry/transaction configuration to handle DB lock
semantics consistently: add DB lock exceptions (e.g., PessimisticLockException,
LockTimeoutException) to notRecoverable or otherwise ensure they are treated the
same as BusinessException (so they are not blindly retried), increase the
backoff window (larger initial delay and maxDelay and/or more attempts) to allow
for realistic lock contention, change propagation from REQUIRES_NEW to REQUIRED
(or remove REQUIRES_NEW) so retries reuse the same transactional context instead
of spawning new transactions, and set an explicit JPA lock timeout on the
repository PESSIMISTIC_WRITE query via
@QueryHints(name="javax.persistence.lock.timeout", value="3000") to avoid
relying on DB defaults.
|
재시도 로직 추가한 코드 수정해서 push했습니다!! |
seongjunnoh
left a comment
seongjunnoh
left a comment
There was a problem hiding this comment.
많이 늦은 리뷰네요 하하
여러 트래픽 생성 테스트 시나리오를 고려하기 위해 부하 테스트 스크립트를 고민하신 부분이 좋네요ㅎ
리뷰 달아놓은 부분 확인부탁드립니다!
| @Override | ||
| @Transactional | ||
| @Retryable( | ||
| notRecoverable = { BusinessException.class, InvalidStateException.class}, |
There was a problem hiding this comment.
p2 : 해당 메서드에서 BusinessException, InvalidStateException이 발생하면 일단 maxAttempts 만큼 재시도가 된 후, exception이 throw 되는것 아닌가요??
코래말처럼 noRetryFor로 변경 후 해당 exception들을 그냥 throw 하는 @recover 메서드를 정의하는 등으로 수정해야 의미없는 재시도를 막을 수 있을것 같습니다!
실제로 FeedChangeLikeStatusApiTest.likeFeed_AlreadyLiked_Fail 테스트 메서드 실행해보니 select for update 쿼리문이 3번 반복된 후, BusinessException을 throw하는것 확인했습니다
| @Lock(LockModeType.PESSIMISTIC_WRITE) | ||
| @Query("SELECT r FROM RecordJpaEntity r WHERE r.postId = :postId") | ||
| Optional<RecordJpaEntity> findByPostIdForUpdate(@Param("postId") Long postId); |
There was a problem hiding this comment.
p2 : 여기도 feed 와 마찬가지로 timeout 추가해도 되지 않나요?
There was a problem hiding this comment.
아이고 제가 피드만 추가했었네요 감사합니닷 레코드 보트 추가하도록하겠습니다!!
| @Lock(LockModeType.PESSIMISTIC_WRITE) | ||
| @Query("SELECT v FROM VoteJpaEntity v WHERE v.postId = :postId") | ||
| Optional<VoteJpaEntity> findByPostIdForUpdate(@Param("postId") Long postId); |
There was a problem hiding this comment.
p2 : 여기도 feed 와 마찬가지로 timeout 추가해도 되지 않나요?
| import org.springframework.boot.autoconfigure.SpringBootApplication; | ||
| import org.springframework.boot.context.properties.ConfigurationPropertiesScan; | ||
| import org.springframework.data.jpa.repository.config.EnableJpaAuditing; | ||
| import org.springframework.retry.annotation.EnableRetry; |
There was a problem hiding this comment.
p3 : 안쓰는 import 문 삭제부탁드립니다!
| @Configuration | ||
| @EnableRetry(proxyTargetClass = true) | ||
| public class RetryConfig { | ||
| } No newline at end of file |
노션 문서 확인했습니다. 테스트 코드에 트랜잭션 설정을 제외하고 수동으로 데이터 클렌징 작업을 수행하는것 좋습니다! |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/konkuk/thip/post/application/service/PostLikeService.java (1)
62-66: 좋아요 수 업데이트 순서가 여전히 검증보다 먼저 수행됩니다.이전 리뷰에서 지적된 문제가 아직 해결되지 않았습니다. 현재 코드는 62-64줄에서 좋아요 수를 업데이트한 후, 67-68줄 또는 73-74줄에서 좋아요/취소 가능 여부를 검증합니다.
검증 실패 시 트랜잭션이 롤백되어 데이터는 보호되지만, 로직 흐름이 부자연스럽고 유지보수성이 저하됩니다. 검증 후에 상태를 변경하도록 순서를 조정하세요.
🔎 권장 수정 방향
// 2. 유저가 해당 게시물에 대해 좋아요 했는지 조회 boolean alreadyLiked = postLikeQueryPort.isLikedPostByUser(command.userId(), command.postId()); - // 3. 게시물 좋아요 수 업데이트 - post.updateLikeCount(postCountService,command.isLike()); - postHandler.updatePost(command.postType(), post); - - // 4. 좋아요 상태변경 + // 3. 좋아요 상태변경 if (command.isLike()) { postLikeAuthorizationValidator.validateUserCanLike(alreadyLiked); postLikeCommandPort.save(command.userId(), command.postId(),command.postType()); - - // 좋아요 푸쉬알림 전송 sendNotifications(command); } else { postLikeAuthorizationValidator.validateUserCanUnLike(alreadyLiked); postLikeCommandPort.delete(command.userId(), command.postId()); } + // 4. 게시물 좋아요 수 업데이트 + post.updateLikeCount(postCountService, command.isLike()); + postHandler.updatePost(command.postType(), post); + return PostIsLikeResult.of(post.getId(), command.isLike());
🧹 Nitpick comments (1)
src/main/java/konkuk/thip/post/application/service/PostLikeService.java (1)
81-84: 락 타임아웃 발생 시 로깅 추가를 권장합니다.재시도 실패 후
RESOURCE_LOCKED예외를 던지기 전에 로깅을 추가하면 운영 환경에서 디버깅에 도움이 됩니다.🔎 로깅 추가 예시
+import lombok.extern.slf4j.Slf4j; +@Slf4j @Service @RequiredArgsConstructor public class PostLikeService implements PostLikeUseCase { ... @Recover public PostIsLikeResult recoverLockTimeout(LockTimeoutException e, PostIsLikeCommand command) { + log.warn("Lock timeout after retries - postId: {}, userId: {}", + command.postId(), command.userId(), e); throw new BusinessException(ErrorCode.RESOURCE_LOCKED); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/konkuk/thip/post/application/service/PostLikeService.javasrc/main/java/konkuk/thip/roompost/adapter/out/persistence/repository/record/RecordJpaRepository.javasrc/main/java/konkuk/thip/roompost/adapter/out/persistence/repository/vote/VoteJpaRepository.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/konkuk/thip/roompost/adapter/out/persistence/repository/vote/VoteJpaRepository.java
- src/main/java/konkuk/thip/roompost/adapter/out/persistence/repository/record/RecordJpaRepository.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
src/main/java/konkuk/thip/post/application/service/PostLikeService.java (3)
3-6: LGTM!재시도 메커니즘과 예외 처리에 필요한 import들이 깔끔하게 정리되었습니다.
Also applies to: 22-27
45-50: 현재 설정이 올바릅니다.PessimisticLockException추가는 불필요합니다.코드베이스에서 모든 리포지토리의
findByIdForUpdate메서드에jakarta.persistence.lock.timeout=3000이 명시적으로 설정되어 있습니다. 이 경우 락 타임아웃 발생 시 Jakarta Persistence는LockTimeoutException을 발생시키므로, 현재retryFor = {LockTimeoutException.class}설정이 정확합니다.PessimisticLockException은 코드베이스에서 사용되지 않으며, 명시적 타임아웃이 설정되지 않은 다른 시나리오에서 발생합니다.
86-94: @recover 메서드들이 정상 작동합니다.Spring Retry의 실제 동작을 확인한 결과,
noRetryFor에 지정된 예외라도notRecoverable에 포함되지 않으면@Recover메서드가 호출됩니다.현재 코드의
@Retry설정에는notRecoverable파라미터가 없으므로,InvalidStateException과BusinessException은 재시도 없이 즉시 발생할 때 해당하는@Recover메서드가 정상적으로 호출됩니다.따라서 이 @recover 메서드들은 데드 코드가 아니며, 불필요한 수정이 없습니다.
오호 문서 잘 봤습니다! 다만, 제가 #337 여기 PR에서 성준님의 답변으로 남겨둔 것처럼 Retryable에 의한 재시도 로직은 propagation을 설정해두지 않아도 새로운 트랜잭션으로 실행되는 것으로 판단됩니다! 따라서, 저는 transactional의 옵션을 따로 부여하지 않고 통합 테스트에서 Transactional 어노테이션도 제거하지 않았어요. 이렇게 되면, 통합 테스트시에 재시도 로직이 발생할 경우는 새로운 트랜잭션으로 실행되지 않겠지만 저희의 통합 테스트 환경에서는 락 타임아웃이 발생하지 않는 환경이니 재시도가 일어날 일이 없다고 판단했습니다! |
# Conflicts: # src/main/java/konkuk/thip/config/RetryConfig.java
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
loadtest/room/room_join_load_test.jssrc/main/java/konkuk/thip/common/exception/code/ErrorCode.java
🧰 Additional context used
🪛 GitHub Actions: CI with Gradle
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
[error] 8-8: ErrorCode is not abstract and does not override abstract method getMessage() in ResponseCode
[error] 36-36: RESOURCE_LOCKED is already defined in enum ErrorCode
#️⃣ 연관된 이슈
📝 작업 내용
https://separate-snowplow-d8b.notion.site/API-294b701eb0b880088501d46c49b434ad?source=copy_link
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
새로운 기능
버그 수정
테스트
✏️ Tip: You can customize this high-level summary in your review settings.